-
-
Notifications
You must be signed in to change notification settings - Fork 428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UsbSerialDiscovery service based on Javax-Usb #3930
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Andrew Fiddian-Green <[email protected]>
This reverts commit 14e3349.
The word 'database' is confusing. So I changed its name to information provider. It is just that in some cases the javax.usb code delivers null values for product and manufacturer description texts. So information provider fills in the blanks for a dozen or so sticks where we happen to already have that information from other sources i.e. the community. |
Can it also be used on Linux instead of linuxsysfs or does it come with significant disadvantages?
Does this match the strings provided by linuxsysfs and ser2net? If not that may cause discovery issues. |
Yes it CAN be used on Linux (but it is not a MUST). Depending on user access rights -- javaxusb may return only the vendorId and productId yet the manufacturer description and product descriptions might be null. I don't know if linuxsysfs suffers from similar access rights issues too, but I get the impression that linuxsysfs might have a higher chance of returning both the ids, and the manufacturer and product descriptions strings too. ?? However TBH I have not really done a full side by side cross comparison of which delivers what data under which circumstances..
I imagine that if both linuxsysfs and javaxusb DO successfully return all strings (rather than null) then those strings would almost certainly be identical. (Note ser2net is not really in the same ball game as the other two, so it might be different -- but I can't really say). In the cases where javaxusb returns null manufacturer / product strings, then the local information provider might be able to fill in those gaps from OH local data -- and in that case the fill-in strings almost certainly WILL differ from the OEM strings (if they would have been retrieved). Reason is that the local fill-in strings (written by me) explicitly have key-words (e.g. Z-Wave, Zigbee, etc.) in them in order to specifically improve the chances of proper discovery.
As mentioned above, my feeling is that the local fill-in information may IMPROVE chances of discovery. Obviously in the case of inbox, the representation property must be stable across discovery service implementations. Which I think it is already. However perhaps you are thinking of some other reasons why this might cause issues? In that case please let me know so I can try to address them. |
Without wishing to further confuse the issue, I should perhaps mention that there is potentially yet another way of extracting USB device discovery data on the Windows platform -- namely via the Windows registry (see below). I don't even know if it is possible for Java code to read the windows registry, but if it is, then I imagine we could parse the registry analog to the way that linuxsysfs does it with the file system. Or something like that. Just a thought.. |
Maybe using this: https://www.rgagnon.com/javadetails/java-read-write-windows-registry-using-jna.html ? |
But that means that other |
Weeell .. it is complicated :(
So the purpose of this local information provider is: if the application specific properties are null, and the chip id properties are known (they are always), and those properties are NOT for a very common generic UART chip, then try to fill in the gaps with a locally sourced version of the application specific properties, which is based on knowledge that we have gathered from our users. Related threads
Related PRs |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
If it will only be used on macOS we can remove the Linux and Windows binaries which will reduce the bundle size from ~1 MB to ~0.2 MB.
I think @kaikreuzer and @J-N-K use Macs so maybe they can tell how well this discovery works on macOS? |
And indeed the features for the sysfs and windowsregistry discovery classes can also perhaps be made platform conditional. Or ?? |
Wouldn’t that mean se have to compile and maintain a copy ourselves? |
The native libraries are dependencies of org.usb4java:usb4java which can be excluded: <dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>linux-x86</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>linux-x86-64</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>win32-x86</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>win32-x86-64</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>darwin-x86-64</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>linux-arm</classifier>
</dependency>
<dependency>
<groupId>org.usb4java</groupId>
<artifactId>libusb4java</artifactId>
<version>${libusb4java.version}</version>
<classifier>linux-aarch64</classifier>
</dependency> |
...t/java/org/openhab/core/config/discovery/usbserial/javaxusb/test/JavaxUsbDiscoveryTests.java
Outdated
Show resolved
Hide resolved
@andrewfg As openhab/openhab-distro#1626 is merged, I tried this on Windows. My zwave stick is found,
zwave addon is included in addons.xml. Should it be suggested, or did I miss something to make it work? |
I think so. But why do you ask? It sounds like you have some doubt. Are you saying that it is 'found' but not 'suggested'? If so can you please trace log the suggestion finder? |
My Zwave stick is discovered by We seem to have a valid description of Zwave add-on in I was just wondering if Zwave add-on should be suggested - for me it is not the case. I have not looked into the code deeply, but if I read #3922 correctly, it should automatically use the new |
Indeed. It will use ALL UsbSerialDiscovery components i.e. the existing Linux 'sysfs' and 'Ser2Net' components plus also my new 'javaxusb` and 'windowsregistry' components.
Hmm. One thought is that the
NOTE: I have the same Z-Wave stick as you So can you please submit trace logs for the finder? |
You may have run into #3983 |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@wborn I am not sure how you want to handle this? |
@wborn these are transitive dependencies and I would not know how to (directly) exclude them. This is a different case from from the feature |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
<overWriteReleases>true</overWriteReleases> | ||
<overWriteSnapshots>true</overWriteSnapshots> | ||
<excludeTransitive>false</excludeTransitive> | ||
<type>jar</type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can exclude the Linux and Windows libraries by adding this:
<type>jar</type> | |
<excludes>**/linux-*,**/linux-*/*.so,**/win32-*,**/win32-*/*.dll</excludes> | |
<type>jar</type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed it only contains darwin-x86-64 libraries so it will not work on Apple silicon based Macs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will not work on Apple silicon based Macs
Nor on Arm based ones .. although TBH I am not sure if any such exist(ed)..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. and I even wonder if MacOS does not (still) have an inner core based on Linux .. I know that it used to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's based on Unix because Linux didn't even exist yet back then. 🦖
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To summarise: this seems to be a Mac specific issue; neither you nor I have a Mac to test on; so we can't really judge if this PR adds value, or if the existing Discovery component added value prior to this. So the question is what shall we do with this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for the feedback of a maintainer using macOS. IIRC there is also x86 emulation on the Macs using Apple silicon. I'm not familiar with how the emulation works. It could be that it only works well when all code runs on the x86 emulator and not just a single library.
@openhab/core-maintainers does any of you have a Mac where you can a) test this PR and/or b) at least have an opinion on it? |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Well, I am on macOS, but if I understand it right, you are looking for someone with Apple Silicon, right? I am still on x86... |
@kai the issue is we have various UsbSerialDiscovery implementation components; we have one that can find usb sticks on Linux, one that can find usb sticks on Windows, and one that can find ip remote usb sticks. However we do not have one that can find usb sticks on any other platform. The implementation in this PR could in theory find usb sticks on any other platform. In fact it can find sticks on Linux and Windows as well, but since it is quite heavy we prefer to use the other existing implementations on those platforms, and only use this PR on the platforms that the other components don't support. However I cannot test if this PR does actually work on such other platforms, so we don't really know if this PR provides any value at all on such platforms. Your use case MacOS on x86 is at least one candidate for this, and MacOS on Apple Silicon would be another. |
It only has darwin-x86-64 libraries, so if it does not work for you it certainly won't work on Apple silicon. 😉 |
I am by now on Apple Silicon, so I guess I do not even have to test this anymore... |
<parent> | ||
<groupId>org.openhab.core.bundles</groupId> | ||
<artifactId>org.openhab.core.reactor.bundles</artifactId> | ||
<version>4.2.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<version>4.2.0-SNAPSHOT</version> | |
<version>4.3.0-SNAPSHOT</version> |
Split from #3922
This is a new implementation of the UsbSerialDiscovery interface based on Javax-Usb. It provides a UsbSerialDiscovery service for all operating systems. So in particular it extends coverage to Windows and Mac OS which are otherwise not covered by an implementation of UsbSerialDiscovery.
Signed-off-by: Andrew Fiddian-Green [email protected]